Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(java): escape oneOf naming #268

Merged
merged 2 commits into from
Mar 21, 2022
Merged

fix(java): escape oneOf naming #268

merged 2 commits into from
Mar 21, 2022

Conversation

shortcuts
Copy link
Member

🧭 What and Why

🎟 JIRA Ticket: https://algolia.atlassian.net/browse/APIC-382

Changes included:

In #227 we can see the Java generator fail due to oneOf types containing special characters.

The oneOf names are using the type name, which in the case of an interface, is correct, but not when native types such as List<..>

🧪 Test

  • CI :D

@shortcuts shortcuts self-assigned this Mar 18, 2022
@shortcuts shortcuts marked this pull request as draft March 18, 2022 08:53
@netlify
Copy link

netlify bot commented Mar 18, 2022

✔️ Deploy Preview for api-clients-automation canceled.

🔨 Explore the source changes: e0294f6

🔍 Inspect the deploy log: https://app.netlify.com/sites/api-clients-automation/deploys/62346119881ee20008337355

@shortcuts
Copy link
Member Author

shortcuts commented Mar 18, 2022

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the generated/main branch.

@shortcuts shortcuts marked this pull request as ready for review March 18, 2022 10:51
@shortcuts shortcuts requested a review from millotp March 18, 2022 10:51
@@ -23,3 +23,5 @@ dist
.openapi-generator

tests/output/*/.openapi-generator-ignore

generators/bin
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is generating this bin ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generators when built locally without the docker image, I'm not sure why it did not happened before

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are built by the scripts inside the docker image, and should only create a .gradle and build folder, do you have something else installed on your IDE ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I don't even have a Java extension on my vscode.

Do you want me to remove it from the gitignore?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No if you have it something is generating it, I just wanted to understand what

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do more tests in #227 later, will let you know if I know what is the issue

@@ -723,7 +723,10 @@ void deleteByTest0() {
}

EchoResponseInterface req = (EchoResponseInterface) assertDoesNotThrow(() -> {
return client.deleteBy(indexName0, SearchParams.of(searchParams0));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised those are the only changes to the test, I think there is something more, I will check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is definitely a missing part with the CTS I think, I still can't make #227 work :(

shortcuts added a commit that referenced this pull request Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants